Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(theme): SASS-based themes #322

Merged
merged 10 commits into from
Oct 28, 2015
Merged

chore(theme): SASS-based themes #322

merged 10 commits into from
Oct 28, 2015

Conversation

redox
Copy link
Contributor

@redox redox commented Oct 21, 2015

What do you think about SASS-based themes?

TODO:

  • color variables
  • autoprefixer -> will do
  • npm shrinkwrap --dev

Ref #290

@vvo
Copy link
Contributor

vvo commented Oct 21, 2015

Never used sass and would prefer to use postcss as proposed in #290.

We can do the exact same PR and see how it goes with postcss.

@redox
Copy link
Contributor Author

redox commented Oct 21, 2015

Never used sass and would prefer to use postcss as proposed in #290.

Happy to compare once someone give postcss a try ;)

@redox
Copy link
Contributor Author

redox commented Oct 21, 2015

We can do the exact same PR and see how it goes with postcss.

Before going for SASS I tried PostCSS and couldn't make it work the way I was imagining it -> so I fallback to what I knew :)

@vvo
Copy link
Contributor

vvo commented Oct 21, 2015

Happy to compare once someone give postcss a try ;)

This will be a 🆚 🎮 :D

@Jerska
Copy link
Member

Jerska commented Oct 21, 2015

Should I do a stylus PR ? ^^


@mixin element($element)
&__#{$element}
@content
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you use that?

Hmm, the goal of BEM is to not have to do any nesting. Sass can let you easily do a lot of nesting, but we should actually never nest more than 2 or 3 levels deep.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, the good thing it that is doesn't nest anything :) The nesting is just use to build the class names from the parent using that &-- and &__ syntax.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. I did not know that syntax.

I'm not sure the end result in Sass will be any more readable than plain CSS, though :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the end result in Sass will be any more readable than plain CSS, though :)

For me, no-brainer :)

/* REFINEMENT LIST */
.ais-refinement-list {
}
.ais-refinement-list--header {
  font-size: 1.2em;
}
.ais-refinement-list--body {
}
.ais-refinement-list--list {
}
.ais-refinement-list--item {
}
.ais-refinement-list--item__active {
  font-weight: bold;
}
.ais-refinement-list--label {
}
.ais-refinement-list--checkbox {
}
.ais-refinement-list--count {
  float: right;
  display: inline-block;
  background: black;
  color: white;
}
.ais-refinement-list--footer {
}

VS

// REFINEMENT LIST
+component(refinement-list)
  +modifier(header)
    font-size: 1.2em
  +modifier(body)
  +modifier(list)
  +modifier(item)
    +element(active)
       font-weight: bold
  +modifier(label)
  +modifier(checkbox)
  +modifier(count)
    float: right
    display: inline-block
    background: black
    color: white
  +modifier(footer)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my point exactly. I find the first one easier to read :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you can use the other mixin:

// REFINEMENT LIST
+bem(refinement-list, header)
  font-size: 1.2em
+bem(refinement-list, body)
+bem(refinement-list, list)
+bem(refinement-list, header)
+bem(refinement-list, item)
+bem(refinement-list, item, active)
     font-weight: bold
+bem(refinement-list, label)
+bem(refinement-list, checkbox)
+bem(refinement-list, count)
    float: right
    display: inline-block
    background: black
    color: white
+bem(refinement-list, footer)

@pixelastic
Copy link
Contributor

I wanted to do some Sass at first but @vvo had a very good point about writing CSS for the future. We would have used a postprocessor for cross-compat anyway, so I'm gonna see if all the variables/mixins can still be replicated in postcss

@vvo
Copy link
Contributor

vvo commented Oct 22, 2015

Careful, we said in #290 that we were investigating this but that should not even be needed for a 1.0.0 release. Themes can just be jsdelivr/default.css+theme.css and that's fine like that.

@redox
Copy link
Contributor Author

redox commented Oct 22, 2015

Careful, we said in #290 that we were investigating this but that should not even be needed for a 1.0.0 release. Themes can just be jsdelivr/default.css+theme.css and that's fine like that.

If we want the community to build theme, we'll need to. For 1.0 I'm happy using SASS and move to anything else if you think it's better.

Regarding the cross-compat, that has been solved years ago with autoprefixer & cie; I'm really not afraid of that. To be honest, I don't see the benefit of PostCSS compared to SASS for what we need.

@pixelastic
Copy link
Contributor

To be honest, I don't see the benefit of PostCSS compared to SASS for what we need.

I would say, the same as using babel instead of coffeescript. Write for the future, in something close to a standard. Feature-wise it will not change much, so ok to do a Sass version first if it's what we're more proficient in.

@vvo
Copy link
Contributor

vvo commented Oct 22, 2015

Sass version first if it's what we're more proficient in.

I cannot read any SASS at all but that's ok if I do not need to :)

@vvo
Copy link
Contributor

vvo commented Oct 22, 2015

If we want the community to build theme, we'll need to.

Right now if you need to build a theme you have to include default.css and then only override the classes you need. Even using SASS or postcss feels too early for me, we haven't built any theme over default.css

This allow you to write things such as:

```sass
+bem(my-widget, item)
  text-align: right
+bem(my-widget, item, active)
  text-align: left
```
@redox
Copy link
Contributor Author

redox commented Oct 22, 2015

Even using SASS or postcss feels too early for me, we haven't built any theme over default.css

Actually, from my POV that's something required to build a theme. We're so used to pre-processors than going back to vanilla CSS is like going back to ES3 :)

@vvo
Copy link
Contributor

vvo commented Oct 22, 2015

Actually, from my POV that's something required to build a theme. We're so used to pre-processors than going back to vanilla CSS is like going back to ES3 :)

Yes I get it, I never used any pre processor to do any CSS file. Only small css files and utilities.

@bobylito
Copy link
Contributor

Not convinced by SASS overall and would prefer a more future proof / flexible approach. I do admit that it seems powerful though. So up to you.

@redox As for the comparaison with ES3, I would say using SASS now feels like starting using coffeescript when ES6 is out...

@redox
Copy link
Contributor Author

redox commented Oct 24, 2015

@redox As for the comparaison with ES3, I would say using SASS now feels like starting using coffeescript when ES6 is out...

Not really, SASS is a really a common use-case: a big majority of front projects are using S{C,A}SS (including bootstrap v4 for instance); on the other hand a very minority of JS projects are using Coffee.

@Shipow
Copy link
Contributor

Shipow commented Oct 24, 2015

I must admit postCSS looks nice, but it also looks a bit more complex and less readable. I haven't tried it yet, who did?
So I'm happy with SASS (surprise) for the first versions.

@vvo
Copy link
Contributor

vvo commented Oct 24, 2015

I haven't tried it yet, who did?

I would like too, we should try to find 1-2 hours to at least try to reproduce what @redox did with postcss + plugins (autoprefixer, mixins).

Also I would like to see what a theme would look like in term of reutilization, I guess @pixelastic you are on it?

redox added a commit that referenced this pull request Oct 28, 2015
chore(theme): SASS-based themes
@redox redox merged commit 0512662 into develop Oct 28, 2015
@redox redox removed the in progress label Oct 28, 2015
@redox redox deleted the sass-theme branch October 28, 2015 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants